Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Compiler plugin] Lower frontend generated implicit receivers #869

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

koperagen
Copy link
Collaborator

For extension properties to work, frontend plugin inserts implicit receivers. But they are just non-existing references needed for resolve. Backend must find them and replace with something that will be executed

@koperagen koperagen added the Compiler plugin Anything related to the DataFrame Compiler Plugin label Sep 18, 2024
@koperagen koperagen self-assigned this Sep 18, 2024
@@ -11,6 +12,7 @@ import org.jetbrains.kotlinx.dataframe.impl.columnName
import kotlin.reflect.KProperty
import kotlin.reflect.typeOf

@HasSchema(schemaArg = 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does HasSchema do again? There's no docs :/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To help identify type parameter that represents dataframe schema
org.jetbrains.kotlinx.dataframe.plugin.InterpretKt#getSchema
org/jetbrains/kotlinx/dataframe/plugin/interpret.kt:255

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, so if we would take the type parameter at schemaArg, which in AggregateDsl<out T> is out T, and put it in DataFrame<> like DataFrame<T>, a schema generated for thát would represent this object. And this could be different for types like GroupedDataRow<out T, out G> where G might represent the data schema?

Could you explain this in a small piece of KDoc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll think of something

fun box(): String {
val df = List(10) { Record(it.toString(), it) }.let { dataFrameOf(*it.toTypedArray()) }
val aggregate = df.pivot { b }.aggregate {
this.add("c") { 123 }.c
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which part failed before and is now solved?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

execution of this expression this.add("c") { 123 }.c failed due to wrong receiver in generated bytecode. c extension property used this@aggregate instead of scope it comes from

@OptIn(UnsafeDuringIrConstructionAPI::class)
private fun IrExpression.replaceWithConstructorCall(): IrConstructorCallImpl {
val constructor = type.getClass()!!.constructors.toList().single()
val type = type
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary assignment

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@koperagen koperagen force-pushed the lower-implicit-receivers branch from 47d623c to 358fcd9 Compare October 2, 2024 18:13
@koperagen koperagen merged commit 3a4a9c1 into master Oct 2, 2024
3 of 4 checks passed
Copy link
Contributor

github-actions bot commented Oct 2, 2024

Generated sources will be updated after merging this PR.
Please inspect the changes in here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compiler plugin Anything related to the DataFrame Compiler Plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants